Skip to content

Conversation

@david-crespo
Copy link
Contributor

Added integration test for existing implementation, change implementation, test continues to pass. Let's definitely give the logic a close look, I did not exactly write it.

Assembling the users and groups into the final shape is done in Rust code. I don't know if there's any way around that.

@david-crespo david-crespo changed the title [4.1/4] SCIM list users and groups in a single query [SCIM 4.1/4] List users and groups in a single query Oct 31, 2025

async move { self.list_users_in_txn(&conn, err, filter).await }
})
.list_users_with_groups(&conn, err.clone(), filter)
Copy link
Contributor

@papertigers papertigers Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I think doing this in a txn is load bearing, is that right @jmpesp ?

Copy link
Contributor Author

@david-crespo david-crespo Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was only because there were 50 queries in there. It can easily be put back.

david-crespo added a commit that referenced this pull request Oct 31, 2025
Base automatically changed from implement_scim to main November 3, 2025 21:11
david-crespo and others added 7 commits November 3, 2025 15:15
This test verifies that listing groups via the SCIM API correctly includes
member information for each group, including groups with multiple members,
groups with a single member, and groups with no members.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Rewrites list_groups_in_txn as list_groups_with_members to use a single
query that fetches both groups and their members in one database roundtrip.
This uses a LEFT JOIN to the silo_group_membership table and aggregates
the results in application code, similar to the list_users_with_groups
implementation.

The transaction wrapper is removed since it's no longer needed - the
function now performs a single read-only query.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants